Repository navigation
fix: keep overlap category names from element 100 on - #4529
Conversation
chrikrah
left a comment
There was a problem hiding this comment.
@jipeng6036-del, approving at 656cbea: the split recovers the category at every index, and the new cases fail without it.
$ python -m pytest -q test_unstructured/test_utils.py
# 656cbea (head)
40 passed in 3.25s
# 67f4cf1 (the PR's parent), suite as on 67f4cf1
37 passed in 0.42s
# head tests, unstructured/utils.py restored from 67f4cf1
FAILED ...keeps_category_past_element_99[100]
FAILED ...keeps_category_past_element_99[1000]
2 failed, 38 passed in 2.15s
$ ruff check unstructured/utils.py test_unstructured/test_utils.py # ruff 0.15.10, the locked version
All checks passed!
# not run: the rest of test_unstructured
non-blocking: the comment at utils.py:503 says type1[3:] only strips a one-digit index, but with the .strip() it also handled 10 to 99, which is why the break starts at 100. It also describes code the diff removes. # Labels are "{ix}. {category}", and ix can have any number of digits. would hold after the merge.
non-blocking: main is at 0.27.26 now. Merged as is, the entry lands inside the released ## 0.27.19 section, so a rebase with a ## 0.27.27 heading and a matching __version__.py bump would fix that, as the other merged PRs did.
@cragwolfe, this closes #4512 and touches no line #4445 edits. Could you take a look?
Signed-off-by: Jasper <241105014+jipeng6036-del@users.noreply.github.com>
chrikrah
left a comment
There was a problem hiding this comment.
@jipeng6036-del, still approving at d98e4e5. The merge with main keeps the split, and both notes are in: the comment now reads true, and the entry sits under its own ## 0.27.27 with the version bumped to match.
$ python -m pytest -q test_unstructured/test_utils.py
# d98e4e5 (head)
40 passed in 2.32s
# head tests, unstructured/utils.py from e2f22b3 (main, the merge parent)
FAILED ...keeps_category_past_element_99[100]
FAILED ...keeps_category_past_element_99[1000]
2 failed, 38 passed in 1.41s
@cragwolfe, the branch is level with main again. Could you take a look?
cragwolfe
left a comment
There was a problem hiding this comment.
Reviewed commit d98e4e5. No actionable findings.
The category parsing matches the generated label format and preserves the full category for indices of 100 and above. Both nesting directions use the corrected values, while overlap detection and existing partial-overlap behavior remain unchanged. The regression test covers single-digit and larger indices through the public entry point.
Tests were assessed as source; no local tests were run. At review time, exact-commit CI had 46 successful checks, 7 still running, and 1 skipped, with no reported failures.
(authored by codex)
Summary
catch_overlapping_and_nested_bboxes()labels each element as"{ix}. {category}".identify_overlapping_or_nesting_case()then dropped a fixed 3 characters to recover the category, which only matches a one-digit index.From element 100 on, leftover digits leaked into
overlapping_elementsandoverlapping_case(. Table, then0. Table). Partitioned document content was unchanged.The category is now taken from the text after the first
". ". Label construction incatch_overlapping_and_nested_bboxes()is unchanged.Tests
python -m pytest test_unstructured/test_utils.py -k overlapping -q— 11 passed (existing overlap cases plus fillers of 0 / 100 / 1000 elements)python -m ruff check unstructured/utils.py test_unstructured/test_utils.pypython -m ruff format --check unstructured/utils.py test_unstructured/test_utils.py CHANGELOG.mdgit diff --checkAI assistance was used to locate the slice and prepare the patch. The overlap cases were run locally.
Fixes #4512
Updated after review: merged current main, moved the changelog entry to 0.27.27 with the matching version bump, and corrected the label comment. All 40 tests in test_unstructured/test_utils.py pass; Ruff check and format pass. The full test suite was not rerun for these metadata/comment updates.